test(flaky): fix unresolved promise causing linux failures #6088
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
|
This should allow use to remove Let's include that with this commit, since they are inherently coupled. |
karanA-aws
pushed a commit
to karanA-aws/aws-toolkit-vscode
that referenced
this pull request
Jan 17, 2025
## Problem aws#6043 To reproduce, add a 5 second delay to the `after` hook at the top level. One way to do this is to insert this at line 84. ``` after(async function () { clock.uninstall() await sleep(5000) }) ``` Despite asserting that the promise rejects within the test, the promise rejects after the test as well. Not entirely sure why this happening. - Tried manually wrapping in try-catch with an `await` instead of `assert.rejects` and it still fails. - Tried wrapping the promise in another promise before passing to `assert.rejects`. ## Solution What does appear to work, is manually handling the callback of the promise. That is, explicitly defining a `then` and `catch` method to assert the rejection, and awaiting the promise at the end of the test to ensure it resolves before the test finishes. Not sure why this works, but I am unable to reproduce the error with this change. ## Notes - `assert.rejects` implementation: https://github.com/nodejs/node/blob/3178a762d6a2b1a37b74f02266eea0f3d86603be/lib/assert.js#L653. Doesn't appear to be the problem because the same is observed when manually wrapping. - `await` docs: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/await --- <!--- REMINDER: Ensure that your PR meets the guidelines in CONTRIBUTING.md --> License: I confirm that my contribution is made under the terms of the Apache 2.0 license.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem
#6043
To reproduce, add a 5 second delay to the
afterhook at the top level.One way to do this is to insert this at line 84.
Despite asserting that the promise rejects within the test, the promise rejects after the test as well. Not entirely sure why this happening.
awaitinstead ofassert.rejectsand it still fails.assert.rejects.Solution
What does appear to work, is manually handling the callback of the promise. That is, explicitly defining a
thenandcatchmethod to assert the rejection, and awaiting the promise at the end of the test to ensure it resolves before the test finishes. Not sure why this works, but I am unable to reproduce the error with this change.Notes
assert.rejectsimplementation: https://github.com/nodejs/node/blob/3178a762d6a2b1a37b74f02266eea0f3d86603be/lib/assert.js#L653. Doesn't appear to be the problem because the same is observed when manually wrapping.awaitdocs: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/awaitLicense: I confirm that my contribution is made under the terms of the Apache 2.0 license.